remove phrase field in Document model #148
Conversation
|
Cool, I think this looks good. I have a vague memory that we have occasionally not had the same values in the name and phrase fields, though that might have been unintentional and something we eventually fixed. Do you remember? In any case, this looks like a good step forward, and hopefully someday we can actually finish off pelias/schema#285 |
|
I also had a vague memory of discussing it, but it would have been 5+ years back, I had a quick look through the blame and found comments mentioning moving from I don't recall the two fields ever being different, maybe in the schema analysis, but not in the model. ref: 4cc3676 |
|
I think I was thinking of #132, which was indeed a bug. So we should be all good. |
missinglink
force-pushed
the
phrase-as-alias-for-name
branch
from
d4cfa21 to
b848c5a
Compare
… name during serialization.
missinglink
force-pushed
the
phrase-as-alias-for-name
branch
from
b848c5a to
8f08b7b
Compare
remove phrase field in Document in favour of duplicating name during serialisation.
historically we've had two separate fields named
nameandphrasewhich correspond to fields containing prefix grams and those which don't, respectively.the two fields contain exactly the same strings, and I've tried over the years to figure out a clean way of combining them into a single field, a trivial task but one which seems impossible to do without introducing breaking changes 😿
this PR makes a change to the
Documentmodel such that there aren't two copies of all the strings being passed around in memory and operated on.a quick look over the diff in this PR shows that we were doing a bunch of CPU work which was duplicatory and unnecessary, plus we're using up more memory storing all the strings twice 🤷♂️ .
@orangejulius could you please sanity check this for me?